trace: Add SOF_IPC_TRACE_DMA_FREE IPC command by ranj063 · Pull Request #4849 · thesofproject/sof

ranj063 · 2021-10-05T23:20:52Z

This will be used to stop the trace DMA and free
its resources. This change is tagged for ABI 3.20.

ujfalusi · 2021-10-07T09:49:15Z

src/trace/dma-trace.c

Should we also put the channel and set d->dc.chan to NULL?
SOF_IPC_TRACE_DMA_PARAMS_EXT will fail if we have a valid channel already.

Unless we also have #4850 along with this patch.

#4850 was merged, so this is good right @ujfalusi ?

I would still put the channel for consistency:

if (d->dc.chan) { dma_stop(d->dc.chan); /* Should we do an error check? */ dma_channel_put(d->dc.chan); d->dc.chan = NULL; }

ok, ill release the channel but error checking would be pointless because trace work is already stopped right?

kv2019i · 2021-10-13T08:43:53Z

src/trace/dma-trace.c

#4850 was merged, so this is good right @ujfalusi ?

lgirdwood

@ujfalusi pls approve if good for you.

lgirdwood · 2021-10-14T13:41:45Z

@ranj063 looks like we have a crash

unknown ipc: unknown command type 0
unknown ipc: debug cmd 0x50000
AddressSanitizer:DEADLYSIGNAL
=================================================================
==22==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x0000005870d2 bp 0x7fffb3ce93f0 sp 0x7fffb3ce93b0 T0)
==22==The signal is caused by a READ memory access.
==22==Hint: address points to the zero page.
SCARINESS: 10 (null-deref)
    #0 0x5870d2 in dma_stop /src/sof/src/include/sof/lib/dma.h:316:21
    #1 0x5870d2 in dma_trace_disable /src/sof/src/trace/dma-trace.c:429:2
    #2 0x55f464 in ipc_dma_trace_free /src/sof/src/ipc/ipc3/handler.c:757:2
    #3 0x55f464 in ipc_glb_trace_message /src/sof/src/ipc/ipc3/handler.c:886:3
    #4 0x55f464 in ipc_cmd /src/sof/src/ipc/ipc3/handler.c:1571:9
    #5 0x55dc56 in LLVMFuzzerTestOneInput /src/sof/tools/oss-fuzz/fuzz_ipc.c:28:2
    #6 0x456b03 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) cxa_noexception.cpp
    #7 0x4562fd in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) cxa_noexception.cpp
    #8 0x457b8b in fuzzer::Fuzzer::MutateAndTestOne() cxa_noexception.cpp
    #9 0x458675 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, std::__Fuzzer::allocator<fuzzer::SizedFile> >&) cxa_noexception.cpp
    #10 0x447d68 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) cxa_noexception.cpp
    #11 0x470bd2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #12 0x7fc4ca4a40b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #13 0x41f68d in _start (build-out/fuzz_ipc+0x41f68d)

DEDUP_TOKEN: dma_stop--dma_trace_disable--ipc_dma_trace_free
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /src/sof/src/include/sof/lib/dma.h:316:21 in dma_stop
==22==ABORTING

ujfalusi · 2021-10-14T13:50:32Z

src/ipc/ipc3/handler.c

Why don't we call this as dma_trace_free() to be consistent?

it balances with trace_dma_enable during ipc_dma_trace_params() no?

ujfalusi · 2021-10-14T13:53:40Z

src/trace/dma-trace.c

I would still put the channel for consistency:

if (d->dc.chan) { dma_stop(d->dc.chan); /* Should we do an error check? */ dma_channel_put(d->dc.chan); d->dc.chan = NULL; }

This will be used to stop the trace DMA and free its resources. This change is tagged for ABI 3.20. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ujfalusi

@ranj063, thank you, looks great!

lgirdwood · 2021-10-15T11:17:33Z

CI showing a presence detect failure on ADL. @marc-hb I've seen this several time today, looks like we may need to filter the cpufreq errors out ? https://sof-ci.01.org/sofpr/PR4849/build10723/devicetest/?model=ADLP_RVP_SDW&testcase=verify-kernel-boot-log

marc-hb · 2021-10-15T16:44:51Z

cpufreq - and more. All boot logs on ADK: thesofproject/sof-test#786

ranj063 requested review from a team, akloniex, dbaluta, lbetlej, lgirdwood, mmaka1 and plbossart as code owners October 5, 2021 23:20

ranj063 requested a review from lyakh October 5, 2021 23:21

ranj063 force-pushed the fix/trace_dma_free branch from 7d4a946 to 28b0dd2 Compare October 6, 2021 00:01

This was referenced Oct 6, 2021

Add new IPC for trace dma free thesofproject/linux#3197

Merged

dma-trace: Allow DMA re-configuration #4850

Merged

ujfalusi requested changes Oct 7, 2021

View reviewed changes

lgirdwood added this to the ABI-3.20 milestone Oct 7, 2021

lgirdwood added the ABI ABI change involved label Oct 7, 2021

kv2019i approved these changes Oct 13, 2021

View reviewed changes

src/trace/dma-trace.c Outdated

Copy link

Collaborator

kv2019i Oct 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#4850 was merged, so this is good right @ujfalusi ?

kv2019i requested a review from ujfalusi October 13, 2021 08:44

lgirdwood approved these changes Oct 13, 2021

View reviewed changes

ujfalusi requested changes Oct 14, 2021

View reviewed changes

trace: Add SOF_IPC_TRACE_DMA_FREE IPC command

390b0d1

This will be used to stop the trace DMA and free its resources. This change is tagged for ABI 3.20. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 force-pushed the fix/trace_dma_free branch from 28b0dd2 to 390b0d1 Compare October 15, 2021 04:43

ujfalusi mentioned this pull request Oct 15, 2021

Minor fixes #4860

Closed

ujfalusi approved these changes Oct 15, 2021

View reviewed changes

lgirdwood merged commit bd918ad into thesofproject:main Oct 15, 2021

ranj063 deleted the fix/trace_dma_free branch October 15, 2021 16:21

marc-hb mentioned this pull request Oct 16, 2021

verify-kernel-kernel-log: skip ADL platforms thesofproject/sof-test#786

Merged

Conversation

ranj063 commented Oct 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented Oct 14, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ujfalusi left a comment

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented Oct 15, 2021

Uh oh!

marc-hb commented Oct 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ranj063 commented Oct 5, 2021 •

edited

Loading